fix: sort discriminator entries by mapping order #1216
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RomanHotsiy
requested changes
Mar 27, 2020
src/services/models/Schema.ts
Outdated
|
|
||
| if (indexLeft < 0 && indexRight < 0) { | ||
| // out of mapping, order by name | ||
| return left.name - right.name; |
There was a problem hiding this comment.
"test" - "test" === NaNMaybe use localeCompare ?
zeapo
force-pushed
the
fix-mapping-out-of-order
branch
from
b3099cc
to
0376644
Compare
zeapo
force-pushed
the
fix-mapping-out-of-order
branch
from
0376644
to
d8b7410
Compare
RomanHotsiy
approved these changes
Mar 27, 2020
RomanHotsiy
changed the title
|
LGTM 👍 |
jancerman
pushed a commit
to Kontent-ai-Learn/kontent-ai-learn-redoc
that referenced
this pull request
Jun 5, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fix for #1111 (in 6e390f9) sadly broke the order of mapping entries.
Given the discriminator:
The result is out of order:
Also, if we remove the
Dogmappings, we end up with theDogclass between the mappings:This PR adds a sort in case of the presence of a mapping. The behavior is quite simple:
Explicit mappings are prioritized to implicit mappings
gives
Explicit mappings are sorted by the order they were defined in the mapping
Implicit mappings are sorted alphabetically among themselves
In case no mapping is defined, the default behavior is kept.